Skip to content

[skip-changelog] Remove python testsuite and the related workflow #2011

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Dec 16, 2022
Merged

[skip-changelog] Remove python testsuite and the related workflow #2011

merged 3 commits into from
Dec 16, 2022

Conversation

MatteoPologruto
Copy link
Contributor

@MatteoPologruto MatteoPologruto commented Dec 15, 2022 •
edited
Loading

Please check if the PR fulfills these requirements

See how to contribute

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)

What kind of change does this PR introduce?

Removal of python infrastructure

What is the new behavior?

Every test that was part of the python testsuite was recently migrated to the new go testsuite. This PR removes the remaining python infrastructure and some tests' leftovers.

Does this PR introduce a breaking change, and is titled accordingly?

No

Other information

TestCoreSearchNoArgs is supposed to fail since the updated url inside test_index.json specificies the path inside the master branch. Since it does not yet exists, it correctly returns error 404 not found.

@MatteoPologruto MatteoPologruto added the topic: infrastructure Related to project infrastructure label Dec 15, 2022
@MatteoPologruto MatteoPologruto self-assigned this Dec 15, 2022
umbynos
umbynos suggested changes Dec 15, 2022
View reviewed changes
Copy link
Contributor

@umbynos umbynos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

arduino-cli/internal/integrationtest/testdata/test_index.json

Line 68 in 869c971

"url": "https://raw.githubusercontent.com/arduino/arduino-cli/master/test/testdata/evil.zip",
evil.zip has to be moved to another dir, otherwise it's gonna break. The tests are passing because the file it's taken from master

@umbynos
Copy link
Contributor

umbynos commented Dec 15, 2022

You can remove also https://github.com/arduino/arduino-cli/blob/master/pyproject.toml and https://github.com/arduino/arduino-cli/blob/master/poetry.lock

@umbynos
Copy link
Contributor

umbynos commented Dec 15, 2022

And adapt

arduino-cli/docs/CONTRIBUTING.md

Lines 164 to 177 in 513f5a1

##### Software requirements for running integration tests:
A working Python environment. Chances are that you already have Python installed in your system, if this is not the case
you can [download][3] the official distribution or use the package manager provided by your Operating System.
Some dependencies need to be installed before running the tests and to avoid polluting your global Python environment
with dependencies that might be only used by the Arduino CLI, to do so we use [Poetry][poetry-website]. First you need
to install it (you might need to `sudo` the following command):
```shell
pip3 install --user poetry
```
For more installation options read the [official documentation][poetry-docs].

@per1234
Copy link
Contributor

per1234 commented Dec 15, 2022 •
edited
Loading

You can remove also https://github.com/arduino/arduino-cli/blob/master/pyproject.toml and https://github.com/arduino/arduino-cli/blob/master/poetry.lock

@umbynos pyproject.toml and poetry.lock are also used for management of tool dependencies that happen to be Python packages. These tools are still used by the project even after removing the Python-based integration test code:

per1234
per1234 requested changes Dec 15, 2022
View reviewed changes
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This workflow and the associated infrastructure should be removed (even though we still have the Python code in docs/siteversion/siteversion.py, that code is maintained in the assets repo so we don't need to do CI on it in this repo):

https://github.com/MatteoPologruto/arduino-cli/blob/remove-python/.github/workflows/check-python-task.yml

The no longer used dependencies should be removed from pyproject.toml and `poetry.lock.

umbynos
umbynos approved these changes Dec 15, 2022
View reviewed changes
Copy link
Contributor

@umbynos umbynos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

look-mum-no-python

@cmaglie
Copy link
Member

cmaglie commented Dec 15, 2022

:-D

@silvanocerza
Copy link
Contributor

silvanocerza commented Dec 15, 2022 •
edited
Loading

Nice. ❤️

per1234
per1234 requested changes Dec 15, 2022
View reviewed changes
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove all these obsolete integration test dependencies:

https://github.com/MatteoPologruto/arduino-cli/blob/remove-python/pyproject.toml#L9-L20

And also remove this obsolete configuration for the "Black" Python formatter tool:

https://github.com/MatteoPologruto/arduino-cli/blob/remove-python/pyproject.toml#L29-L31

@cmaglie cmaglie changed the title [skip-changelog]Remove python testsuite and the related workflow [skip-changelog] Remove python testsuite and the related workflow Dec 15, 2022
per1234
per1234 approved these changes Dec 15, 2022
View reviewed changes
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Matteo!

@per1234 per1234 added the type: enhancement Proposed improvement label Dec 16, 2022
@MatteoPologruto MatteoPologruto merged commit b6cc517 into arduino:master Dec 16, 2022
@MatteoPologruto MatteoPologruto deleted the remove-python branch December 16, 2022 09:03
@MatteoPologruto MatteoPologruto mentioned this pull request Dec 16, 2022
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@per1234 per1234 per1234 approved these changes

@umbynos umbynos umbynos approved these changes

@cmaglie cmaglie Awaiting requested review from cmaglie

Assignees

@MatteoPologruto MatteoPologruto

Labels
topic: infrastructure Related to project infrastructure type: enhancement Proposed improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@MatteoPologruto @umbynos @per1234 @cmaglie @silvanocerza